Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rofi-games: init at 1.10.2 #301397

Merged
merged 1 commit into from
Oct 28, 2024
Merged

rofi-games: init at 1.10.2 #301397

merged 1 commit into from
Oct 28, 2024

Conversation

TomaSajt
Copy link
Contributor

@TomaSajt TomaSajt commented Apr 4, 2024

Description of changes

Closes: #301374
Related: Rolv-Apneseth/rofi-games#19

This PR adds 1 package: rofi-games

  • the package builds a rofi plugin: (the .so file)
  • and it also includes the recommended rofi themes

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@TomaSajt TomaSajt force-pushed the rofi-games branch 2 times, most recently from 5f50317 to 9212b98 Compare April 4, 2024 13:25
@TomaSajt TomaSajt changed the title rofi-games: init at 1.6.7 rofi-games: init at 1.6.8 Apr 4, 2024
@aikooo7
Copy link
Contributor

aikooo7 commented Apr 6, 2024

Result of nixpkgs-review pr 301397 run on x86_64-linux 1

1 package built:
  • rofi-games

@aikooo7 aikooo7 added the 12.approvals: 2 This PR was reviewed and approved by two reputable people label Apr 6, 2024
@wineee
Copy link
Member

wineee commented Apr 8, 2024

Can we use 'just' to install ? there are some '/usr' path need patch
https://github.com/Rolv-Apneseth/rofi-games/blob/e4dca5222133c7c8956c42cc58b6dd27d5e8656c/justfile#L17

@TomaSajt
Copy link
Contributor Author

TomaSajt commented Apr 8, 2024

Nixpkgs has a custom way of installing rofi plugins:

if you don't have home-manager, then you can change

environment.systemPackages = [
  ...
  pkgs.rofi
];

to

environment.systemPackages = [
  (pkgs.rofi.override { plugins = [ pkgs.rofi-games ]; })
];

It's a bit easier if you have home-manager:
Add this to your home manager config:

programs.rofi = {
  enable = true;
  plugins = [ pkgs.rofi-games ];
};

You will also need to select the theme when calling rofi

Obviously pkgs.rofi-games doesn't yet exist outside of this PR, so you might need to use overlays, or any other method.
Here's an example:
first copy the file from this PR to your config directory and set plugins accordingly:

plugins = [ (pkgs.callPackage ./rofi-games.nix {}) ];

I haven't tested what I've written here, but should be correct.

@Aleksanaa
Copy link
Member

I don't quite understand how it remains binary compatible with rofi. For example, rofi and rofi-wayland are currently incompatible, so all libraries written in C need to be built again with rofi-wayland instead of rofi as a dependency. But this package does not introduce any additional dependencies. The rofi-mode it relies on also doesn't seem to have anything to do with a build of rofi.

@TomaSajt
Copy link
Contributor Author

TomaSajt commented Apr 9, 2024

I don't quite understand how it remains binary compatible with rofi. For example, rofi and rofi-wayland are currently incompatible, so all libraries written in C need to be built again with rofi-wayland instead of rofi as a dependency. But this package does not introduce any additional dependencies. The rofi-mode it relies on also doesn't seem to have anything to do with a build of rofi.

Apparently this line was supposed to do the version detection of rofi, conditionally setting the rofi_next flag.
https://github.com/Rolv-Apneseth/rofi-games/blob/e4dca5222133c7c8956c42cc58b6dd27d5e8656c/justfile#L30
however, I'm doing a custom build and install, so it doesn't trigger.

Should I just introduce a .override-able flag or should I expect the rofi to be passed with .override and use the detection in the justfile?

@Aleksanaa
Copy link
Member

Got a solution.

@Aleksanaa
Copy link
Member

Aleksanaa commented Apr 9, 2024

  # Get rofi version from justfile to ensure abi compatibility
  preBuild = ''
    export RUSTFLAGS+=" $(${lib.getExe just} --evaluate RUSTFLAGS)"
  '';

  nativeBuildInputs = [
    pkg-config
    rofi
  ];

Isn't pretty, though... You can also use just to control the build process.

@TomaSajt
Copy link
Contributor Author

TomaSajt commented Apr 9, 2024

  # Get rofi version from justfile to ensure abi compatibility
  preBuild = ''
    export RUSTFLAGS+="$(${lib.getExe just} --evaluate RUSTFLAGS)"
  '';

  nativeBuildInputs = [
    pkg-config
    rofi
  ];

Isn't pretty, though... You can also use just to control the build process.

Thanks, I'll push a commit shortly which just uses just and patches the justfile. Good to know you can do --evaluate.

@Aleksanaa
Copy link
Member

just should have some command line options to edit or override those args. Will look into it tomorrow.

@TomaSajt
Copy link
Contributor Author

TomaSajt commented Apr 9, 2024

just should have some command line options to edit or override those args. Will look into it tomorrow.

The build phase only calls just with the justFlags as the inputs.

just "${flagsArray[@]}"

However, justFlags is used in all other phases too, which would mean that the install phase would do just build install, which is weird

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin and removed 10.rebuild-darwin: 1 10.rebuild-darwin: 1-10 labels Apr 9, 2024
@wegank wegank removed the 12.approvals: 2 This PR was reviewed and approved by two reputable people label Apr 9, 2024
@wegank wegank added the 12.approvals: 3+ This PR was reviewed and approved by three or more reputable people label May 22, 2024
@Rolv-Apneseth
Copy link

Hey, not sure if anything needs to be done on this end, but just wanted to give a heads up that I have since pushed some changes to rofi-games. Probably the only one that matters for this PR is support for a user config file at $XDG_CONFIG_HOME/rofi-games/config.toml.

@TomaSajt TomaSajt changed the title rofi-games: init at 1.6.8 rofi-games: init at 1.9.1 Jun 5, 2024
@TomaSajt
Copy link
Contributor Author

TomaSajt commented Jun 5, 2024

Hey, not sure if anything needs to be done on this end, but just wanted to give a heads up that I have since pushed some changes to rofi-games. Probably the only one that matters for this PR is support for a user config file at $XDG_CONFIG_HOME/rofi-games/config.toml.

Thanks for the heads up!

@wegank wegank removed the 12.approvals: 3+ This PR was reviewed and approved by three or more reputable people label Jun 20, 2024
@TomaSajt TomaSajt changed the title rofi-games: init at 1.9.1 rofi-games: init at 1.9.2 Jun 25, 2024
@TomaSajt TomaSajt force-pushed the rofi-games branch 2 times, most recently from 94007eb to 0507464 Compare August 19, 2024 11:34
@TomaSajt TomaSajt changed the title rofi-games: init at 1.9.2 rofi-games: init at 1.10.2 Aug 19, 2024
@Aleksanaa Aleksanaa merged commit 3c52fbe into NixOS:master Oct 28, 2024
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Package request: Rofi-games
8 participants